test: Unit tests for Function App Service #156

tbarlow12 · 2019-05-31T22:07:34Z

Resolves [AB#18737]

wbreza

Added some feedback. Let me know if you have any questions.

wbreza · 2019-05-31T22:23:02Z

src/plugins/login/loginPlugin.test.ts

    AzureLoginService.servicePrincipalLogin = servicePrincipalLogin;

    const sls = MockFactory.createTestServerless();
+    delete sls.variables['azureCredentials']


Can we move these into a beforeEach or similar to keep it DRY?

The only issue with that is the deletion of the credentials is specific to these tests only. Other tests depend on the credentials still being there

wbreza · 2019-05-31T22:23:16Z

src/services/baseService.ts

    }
  }

+  protected log(message: string) {


wbreza · 2019-05-31T22:25:44Z

src/test/mockFactory.ts


-  private static createTestService(): Service {
+  public static createTestFunctionApp() {
    return {


We should add return typings here.

wbreza · 2019-05-31T22:27:15Z

src/services/functionAppService.test.ts

+    webAppDelete = jest.fn();
+    sendFile = jest.fn();
+
+    WebSiteManagementClient.prototype.webApps = {


These methods can be mocked by mocking the sub object directly. ex) WebApp.prototype.get = jest.fn(...)

wbreza · 2019-05-31T22:29:17Z

src/services/functionAppService.test.ts

+  beforeAll(() => {   
+
+    // TODO: How to spy on defaul exported function?
+    const axiosMock = new MockAdapter(axios);


Do you find this mock adapter useful? I have mixed feelings.

Yes and no... If I could make assertions on it, it would be amazing. I had to do something fairly hacky to validate the syncTriggers. I wasn't able to spy on axios directly. Maybe we can chat more about how to do that

wbreza · 2019-05-31T22:30:28Z

src/test/mockFactory.ts

+    } as any as Service;
+  }
+
+  public static createTestFunctions(functionCount = 3) {


We should add typings to these mock factory functions.

Part of the reason for not having typings is my own not knowing what types they are 😆

But agreed. I'll do some more digging here

wbreza

Saw a couple things that need attention before merging.

wbreza · 2019-06-07T17:39:14Z

src/models/index.ts

@@ -0,0 +1,4 @@
+export * from "./azureProvider";


Why the exports?

To make model imports easy. I can remove these

wbreza · 2019-06-07T17:40:01Z

src/models/serverless.ts

@@ -0,0 +1,8 @@
+export interface ServerlessYml {


I would remove Yml and maybe rename to ServerlessAzureConfig or similar.

wbreza · 2019-06-07T17:42:15Z

src/plugins/package/azurePackage.test.ts

  it("sets up provider configuration", async () => {
    const slsFunctionConfig = MockFactory.createTestSlsFunctionConfig();
    const sls = MockFactory.createTestServerless();
-    Object.assign(sls.service, {


How do the functions config set if you are removing this?

Looks like with the updates to mock factory you need to now pass into createTestServerless ??

For this instance in particular, since it's just using the createTestSlsFunctionConfig out of the box, that will already be in the sls object. If there is a different function config to pass, that can be achieved by doing:

createTestServerless({ service: createTestService(functionConfig) })

wbreza · 2019-06-07T17:44:39Z

src/services/functionAppService.ts

  }

-  public async getMasterKey(functionApp) {
+  public async getMasterKey(functionApp?) {


Add typeref for functionApp

tbarlow12 · 2019-06-07T19:12:13Z

src/services/apimService.test.ts


  beforeEach(() => {
    const slsConfig: any = {
+      ...MockFactory.createTestService(MockFactory.createTestSlsFunctionConfig()),


@wbreza this is another option for using a custom config

PIC123

Looks good! Much cleaner now too!

Resolves [AB#18737]

tbarlow12 added 5 commits May 31, 2019 14:58

test: Function App Service tests

44a4ee7

style: Update to use double quotes

9c29874

feat: Added log statement in base service

b2da1fc

fix: Remove extra files from different PR

48f7882

Fix: return value from sync triggers

4176705

wbreza approved these changes May 31, 2019

View reviewed changes

tbarlow12 added 4 commits June 3, 2019 07:09

Condense tests to make more DRY

41b9c43

Invoke login hook helper function

7eee80a

Add minimal models and set as return types in mock factory

5ef08e4

Add models

ede2161

tbarlow12 force-pushed the tabarlow/function-app-service-tests branch from 74304a8 to ede2161 Compare June 4, 2019 16:38

tbarlow12 added 6 commits June 4, 2019 10:40

Cleanup

cc2c04b

Resolved conflicts with dev

9db6911

Fix all tests

3df65bf

Clean up

0b1a8c5

Fixed mock factory to use same function structure

f992883

Remove unnecessary service assignment

a30c1f2

wbreza approved these changes Jun 7, 2019

View reviewed changes

Respond to PR feedback

87aebf3

tbarlow12 commented Jun 7, 2019

View reviewed changes

PIC123 approved these changes Jun 12, 2019

View reviewed changes

tbarlow12 merged commit bf2a978 into dev Jun 12, 2019

tbarlow12 deleted the tabarlow/function-app-service-tests branch June 12, 2019 20:29

tbarlow12 added a commit that referenced this pull request Sep 13, 2019

test: Unit tests for Function App Service (#156)

2174184

Resolves [AB#18737]

test: Unit tests for Function App Service #156

test: Unit tests for Function App Service #156

Uh oh!

Conversation

tbarlow12 commented May 31, 2019

Uh oh!

wbreza left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wbreza left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PIC123 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants